Skip to content

Conversation

@yaahc
Copy link
Member

@yaahc yaahc commented Oct 20, 2025

currently mostly a skeleton of a draft so we can collaboratively massage it into shape more easily before filling in with proper reference verbiage.

hoping to take a significant chunk out of #568

@traviscross
Copy link
Contributor

Thanks for posting. Lot of good here. The main thing I'd suggest, by way of helping to sharpen this up, would be to try to write a concise example after each claim, in as many cases as that might make sense, that demonstrates that the claim is true (and would not pass if the claim were false). Aside from the intrinsic benefit of having such examples, I think this might help to focus the text on the language-level effects. (It's not surprising, given the good research you've been doing, that some bits of this currently have some "description of the implementation" flavor.)

Copy link
Member Author

@yaahc yaahc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a fair number of comments are also contained inline in the documents


r[names.resolution.early.imports.shadowing]

The following is a list of situations where shadowing of use declarations is permitted:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The following is a list of situations where shadowing of use declarations is permitted:
The following is a list of situations where shadowing of use declarations is not permitted:

?

Copy link
Member Author

@yaahc yaahc Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original text is correct.

Use glob shadowing: https://doc.rust-lang.org/nightly/reference/items/use-declarations.html#r-items.use.glob.shadowing
macro textual scope shadowing: https://doc.rust-lang.org/nightly/reference/macros-by-example.html#r-macro.decl.scope.textual.shadow

In my mind the "not permitted" set is the list of ambiguity errors below

* .visitation-order
* derive helpers
* not visited when resolving derive macros in the parent scope (starting scope)
* derive helpers compat
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as above about this being deprecated and removed next year.

@rustbot

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Oct 21, 2025
@traviscross
Copy link
Contributor

@ehuss and I are looking at this together on the lang-docs office hours call, and we just wanted to express our appreciation to @petrochenkov for having been so responsive with @yaahc on working out the details here. This is a chapter that we've long wanted to exist, and we're thrilled and appreciative that @yaahc is digging in to shape this up.

@yaahc yaahc force-pushed the name-res branch 6 times, most recently from 5397d08 to 1bd3afe Compare October 29, 2025 21:29
@yaahc yaahc force-pushed the name-res branch 9 times, most recently from 80fd707 to 570bedd Compare November 7, 2025 22:43
* derive helpers used before their associated derive may not shadow other attributes or other derive helpers that are otherwise in scope after their derive
* TODO example? This ones harder to do concisely afaik

Helper attributes may not be used before the macro that introduces them.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • What happens if two macros introduce the same helper, will the second one not
    be able to see the attribute of the first anymore, assuming their order is
    "firstmacro" "helper" "secondmacro"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will the second one not
be able to see the attribute of the first anymore

It will, helpers are not removed.
Helpers don't have "identity" really, technically the compiler tracks it, but for the compiler it's important that it's someone's helper and that's enough, and proc macros detect helpers by string comparison, so they'll happily use someone else's helpers with the same name as their own.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to add a test for this to rust-lang/rust, if there's none currently.

Copy link
Member Author

@yaahc yaahc Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was setting up a test for this and I think I found a mistake I made in interpreting the logic in resolve_ident_in_scope_set. Specifically I said

                                } else if innermost_res == derive_helper_compat
                                    || res == derive_helper_compat && innermost_res != derive_helper
                                {
                                    Some(AmbiguityKind::DeriveHelper)

means

  • if a derive helper which is used before it's derive has the same name as an outer item
  • or if a derive helper has the same name as an outer derive helper which is used before it's derive

The first bullet is true afaict, being in this codepath implies theres a res and innermost_res, so having the innermost be a derive_helper_compat means that we have a second candidate we found later in the same sub-namespace.

The second bullet is incorrect. It seems like I interpreted the != as an ==. This is just saying that the second resolution is a compat resolution, and the first resolution is not a helper, which means it could only be another helper compat due to visitation order. Given the lack of identity on helpers, I'm not sure how to trigger this second half of the condition.

Here's the test I setup:

#[empty_helper]
#[derive(Empty)]
#[empty_helper]
#[derive(Empty2)]
struct S;

(edited next two paragraphs)

Both derives introduce the same #[empty_helper] attribute.

Assuming the language I drafted in this PR which this comment is attached to is correct, the first #[empty_helper] should produce a warning and the second one should produce an error, since Empty2's helper attribute is shadowing the correctly in scope attribute from Empty. Instead, the only error this produces is the future incompat warning on the first helper for using a helper before it's derive which is converted into an error by deny(future_incompat).

I'm going to try digging into this a bit deeper to see if I can pinpoint exactly when this subbranch gets triggered or whether I'm correct in thinking this is inert.

https://github.com/rust-lang/rust/pull/79078/files#diff-c75db064f0f7310431b21d0ecb61c9438c6e48f9ab2810a25251f504d21c6d9dR865 introduced this logic

Edit: looking at visit_scopes it looks like we're never "iterating through derive_helper_compat" scopes so I think it will only ever call the closure from resolve_ident_in_scope_set once for the derivehelpercompat scope and only ever produce one derive_helper_compat candidate, so the only candidates that could be visited earlier are the proper derivehelper candidates.

I went ahead and added a panic in the second case and I'm checking if that causes our testsuite to fall over anywhere but initial indications (running it in tests/ui/proc-macro) are coming up clean, implying it is in fact inert.

@yaahc yaahc force-pushed the name-res branch 2 times, most recently from 7e17934 to 4744e19 Compare November 20, 2025 19:47
@yaahc yaahc force-pushed the name-res branch 3 times, most recently from 9ada73e to f982337 Compare November 20, 2025 21:48

r[names.namespaces.sub-namespaces.use-shadow]
It is still an error for a [`use` import] to shadow another macro, regardless of their sub-namespaces.
* TODO revisit
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisiting this. I think I've decided against wanting to move the entire sub-namespaces section into the name resolution chapter. I was consider if it made sense to argue that they aren't a true namespace since the mechanism through which they're implemented is different and that they're actually a property of the name resolution algorithm that creates "apparent sub-namespaces" or something along those line. I ended up convincing myself that this is a distinction without a difference and its just simpler pedagogically to call them sub-namespaces.

I do think I want to give the use-shadowing subsection the same treatment as the other ambiguity errors, remove the section here and add an admonition linking to the relevant section in name-resolution.

```
r[names.resolution.early.imports.ambiguity.pathvstextualmacro]
Path-based scope bindings for macros may not shadow textual scope bindings to macros. For bindings from [use declarations], this applies regardless of their [sub-namespace].
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implication here that I need to investigate: that it is possible to have path-based bindings to macros that would shadow textual bindings if imported but not if otherwise defined in the same scope.

Comment on lines +310 to +318
Macros are resolved by iterating through the available scopes until a candidate
is found. Macros are split into two sub-namespaces, one for bang macros, and
the other for attributes and derives. Resolution candidates from the incorrect
sub-namespace are ignored. The available scopes are visited in the following order.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not at all happy with this, in particular the first sentence seems like a gross oversimplification. I wrote this as is because I was trying to find some way to introduce the scope visitation order logic.

@yaahc yaahc marked this pull request as ready for review November 20, 2025 22:22
@rustbot rustbot added the S-waiting-on-review Status: The marked PR is awaiting review from a maintainer label Nov 20, 2025

r[names.resolution.intro]

_Name resolution_ is the process of tying paths and other identifiers to the declarations of those entities. Names are segregated into different [namespaces], allowing entities in different namespaces to share the same name without conflict. Each name is valid within a [scope], or a region of source text where that name may be referenced. Access to certain names may be restricted based on their [visibility].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What other identifiers?
Single-segment paths are also paths.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lifetimes / type parameters come to mind as an example which I wouldn't consider paths that need their names resolved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lifetimes are not paths, true.
I actually haven't seen this mentioned in the added text, but that's because named lifetimes are resolved in later resolution, which is not documented yet.

Type parameter use is a single-segment path, you don't know that it's a type parameter until you resolve that path.

I recalled one more example though, names of trait impl items are resolved to trait items, and they are not paths.

trait Trait { fn f(); }
impl Trait for () { fn f() {} } // `f` is resolved to its trait origin (during late resolution)

## Ambiguities

r[names.resolution.expansion.imports.ambiguity.intro]
Some situations are an error when there is an ambiguity as to which name a `use` declaration refers. This happens when there are two name candidates that do not resolve to the same entity where neither import is [permitted] to shadow the other.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some situations are an error when there is an ambiguity as to which name a `use` declaration refers. This happens when there are two name candidates that do not resolve to the same entity where neither import is [permitted] to shadow the other.
Some situations are an error when there is an ambiguity as to which name a `use` declaration or a macro path refers. This happens when there are two name candidates that do not resolve to the same entity where neither import is [permitted] to shadow the other.

I think all the cases below are relevant for both.

Copy link
Member Author

@yaahc yaahc Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are correct. I went ahead and added the extra cases and improved the examples for globvsouter to better demonstrate the variety of cases. I also reworded it to clarify that the ambiguities come from the definitions and are found when resolving names for macro invocations or use declarations. This let me also capture that modules can be a source of ambiguity.

@yaahc yaahc force-pushed the name-res branch 5 times, most recently from 6689d3b to 8a6f849 Compare November 21, 2025 20:42
Co-authored-by: Vadim Petrochenkov <[email protected]>
@yaahc yaahc changed the title Expand name resolution stub Add section on expansion-time (early) name resolution Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. S-waiting-on-review Status: The marked PR is awaiting review from a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants